Skip to content
This repository has been archived by the owner on Aug 26, 2021. It is now read-only.

Don't require entry point classes to have @Inject annotations. #57

Merged
merged 1 commit into from Oct 11, 2012

Conversation

swankjesse
Copy link
Member

This is necessary so that generic framework code can inject arbitrary
entry points (like JUnit test cases or Android activities) without
concern for whether the specific entry point has any @Inject annotations.

This is necessary so that generic framework code can inject arbitrary
entry points (like JUnit test cases or Android activities) without
concern for whether the specific entry point has any @Inject annotations.
@cgruber
Copy link
Collaborator

cgruber commented Oct 11, 2012

Can you give an example? I'm not sure what you mean by "generic framework code can inject".

pforhan added a commit that referenced this pull request Oct 11, 2012
Don't require entry point classes to have @Inject annotations.
@pforhan pforhan merged commit 7222847 into master Oct 11, 2012
@swankjesse
Copy link
Member Author

@cgruber some of our tests have a test runner that injects the test case:

@Override public void beforeTest(Method method) {
  makeObjectGraph().inject(this);
}

This is especially useful for integration testing.

Unfortunately we use the same test runner for several tests, and sometimes the injected test doesn't have any @Inject annotated fields. We need to avoid crashing in that case.

@cgruber
Copy link
Collaborator

cgruber commented Oct 11, 2012

So inject() in this case is basically doing a no-op? I can see the utility, but it feels unsafe - like someone will pass in something with no @Inject and end up not realizing until run-time that nothing happened.

This feels like an alternate graph implementation with different behaviour suitable for testing, to be honest. I'm not sure this behaviour is the right one in all cases.

@cgruber
Copy link
Collaborator

cgruber commented Oct 11, 2012

Or an injector with some configuration points with strict defaults.

@swankjesse
Copy link
Member Author

Yeah, the inject() call is a no-op. For us it's come up in several places, mostly in framework code that blindly injects an instance of an abstract type like TestCase or Activity.

Since the offending type still needs to be explicitly registered as an injection point, it's hard to be surprised by this.

@cgruber
Copy link
Collaborator

cgruber commented Oct 11, 2012

Ah - fair point. Ok... as long as we keep some explicit signal there and don't end up with some situation where someone can trip over it. Still less strict than I'd like, but you're right - the class is at least deliberately added.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants